Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internationalize the GUI #2749

Merged
merged 2 commits into from
Jun 10, 2019
Merged

Internationalize the GUI #2749

merged 2 commits into from
Jun 10, 2019

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Apr 30, 2019

Motivations

  • The CKAN GUI is English-only
  • We're split on whether it's en-GB/en-AU or en-US

Changes

  • All user visible properties in the .Designer.cs files are moved into the corresponding .resx files and set via resources.ApplyResources.
  • All user visible strings in code are moved to Properties/Resources.resx and accessed via Properties.Resources.NameOfString.
  • All occurrences of American spelling in the default locale are changed to UK/Aussie spelling.
  • Supplemental .en-US.resx files are added as needed to switch to American spellings for American users.
  • The build process is updated to bundle the "satellite assemblies" that the compiler creates into the main EXE.
  • New classes SingleAssemblyResourceManager and SingleAssemblyComponentResourceManager extend the built in resource manager classes to support bundled satellite assemblies.

Now when a UK/Aussie user opens the CKAN GUI, they'll see familiar extra "u"s and words ending in "ise", while Americans will see no extra "u"s and words ending in "ize".

German translation

@DasSkelett has translated the entire GUI into German!

image

Erfolg!

Icons and exe size

While poking around at our resource files, @DasSkelett discovered that we have 11 copies of the CKAN icon (roughly one per form), and that each one takes up something like 700 KiB, adding up to fully half the size of the compiled exe!!

Now these are moved from the forms' resx files to the project resx file, where one copy is shared. This makes ckan.exe less than half the size it was previously, while working exactly the same.

Known limitations

Only GUI is updated! This PR doesn't touch Core (but a future one might). Lots of visible strings come from Core, but the ones from GUI should all be localizable.

@HebaruSan HebaruSan added Enhancement New features or functionality GUI Issues affecting the interactive GUI Pull request Build Issues affecting the build system labels Apr 30, 2019
@Olympic1 Olympic1 removed the Enhancement New features or functionality label Apr 30, 2019
@HebaruSan HebaruSan added the Enhancement New features or functionality label Apr 30, 2019
@DasSkelett
Copy link
Member

DasSkelett commented Apr 30, 2019

Wow, nice! Did you use the VisualStudio functionality to move all the strings?
Should I open a PR to your branch for the German translation, or create a new PR when this one is merged?

@HebaruSan
Copy link
Member Author

I used Atom and my mouse to move all the strings 🤕, but I based the changes on a demo from Microsoft, so it should match up with how VS would do it:
https://code.msdn.microsoft.com/CSWinFormLocalization-59675a87

As for a Deutsch PR, either way is fine with me. I'd be happy to include it here or add it later.

@DasSkelett
Copy link
Member

DasSkelett commented Apr 30, 2019

Wow, that must have been a lot of work.
Okay, I will do the PR to your branch then, since I found some things to correct (missing strings, formatting...), I can include them in my PR this way.

@DasSkelett
Copy link
Member

DasSkelett commented Apr 30, 2019

Just an idea: What about an option to set the language manually? Can imagine some people wanting it.

And a FYI: You can set the localization yourself in the code if you add those 3 lines (found here) as early as possible (before any localized strings are loaded), e.g. right at the start of GUI.Main():

System.Globalization.CultureInfo ci = new System.Globalization.CultureInfo("en-US");
System.Globalization.CultureInfo.DefaultThreadCurrentCulture = ci;
System.Globalization.CultureInfo.DefaultThreadCurrentUICulture = ci;

If you want to test the German translation for example later on. If you found a better way to do this, I'm all ears.

@HebaruSan
Copy link
Member Author

If you found a better way to do this

That was exactly how I did it. :) Except I only set one of those.

@politas
Copy link
Member

politas commented May 1, 2019

Wow, massive piece of work there, @HebaruSan! Is this ready for our point release, or shall we leave it for the next one?

@HebaruSan
Copy link
Member Author

HebaruSan commented May 1, 2019

I think it's pretty safe. If there are bugs, they should be of the "this dialog has a strange string here" variety rather than crashes or corrupted installs.

@DasSkelett DasSkelett added the In progress We're still working on this label May 1, 2019
@DasSkelett
Copy link
Member

DasSkelett commented May 3, 2019

Can somebody by any chance tell me, why my tranlations work when compiling in VisualStudio (2019), but not when when compiling with the BuildTools (15) installed without VS on Windows, nor when compiling on Linux with mono?
https://github.com/DasSkelett/CKAN/tree/feature/german-translation

And btw, does anybody know how to prevent VS from overwriting all the auto-generated comments?
(See here for example)

@HebaruSan
Copy link
Member Author

The en-US translations seem to be working for me on Mono (after the commit I just pushed):

image

I'll take a look at your branch shortly...

GUI/Main.resx Outdated Show resolved Hide resolved
@DasSkelett
Copy link
Member

DasSkelett commented May 3, 2019

The en-US translations seem to be working for me on Mono (after the commit I just pushed):

Sorry, forgot an important bit: The normal 'per form' translations work perfectly fine, just the translations of Properties/Resources.de-DE.resx (so 'dynamic' strings) are missing, the default text is shown.

@HebaruSan
Copy link
Member Author

The Properties/Resources.en-US.resxseems OK on Mono as well (this would say "favourites" otherwise):

image

@HebaruSan
Copy link
Member Author

Yeah, I'm seeing some weird stuff in de-DE.

image

I'll let you know if I can figure out why...

@DasSkelett
Copy link
Member

DasSkelett commented May 3, 2019

Yeah, I'm seeing some weird stuff in de-DE.

Oh that's probably due to mono/mono#12643.
Problem is: I have to use ContentAlignment.MiddleCenter, else one language will always be off-center.

The Properties/Resources.en-US.resxseems OK on Mono as well (this would say "favourites" otherwise):

Well, it does say 'favourites' on my system (with en-US)...
Seems I changed something (probably in CKAN-GUI.csproj) by accident. You tested on my branch, right?
That's weird...

@HebaruSan
Copy link
Member Author

BTW, in case it's useful, my command for testing on Linux is:

LC_ALL=de_DE _build/ckan.exe

@HebaruSan
Copy link
Member Author

Oh that's probably due to mono/mono#12643.

Huh, you're right. I have a local build of mono's current HEAD (which includes that PR), and it works there:

image

@HebaruSan
Copy link
Member Author

You tested on my branch, right?

My "favourites" test was on my branch, to establish a baseline of whether there's a known-working version here. I'm checking your branch now.

@HebaruSan
Copy link
Member Author

This is where Resources.de-DE.resx broke, it needs to be a SingleAssemblyResourceManager object or it won't check the satellite assemblies bundled in the EXE:

https://github.com/DasSkelett/CKAN/blob/ceae2c58b7f70881fc7eaa0921af0a14f871da9d/GUI/Properties/Resources.Designer.cs#L42

Can you revert all your changes to that file? You should be able to use my version without changes.

@DasSkelett
Copy link
Member

Huh, you're right. I have a local build of mono's current HEAD (which includes that PR), and it works there:

I recognize the issues I fixed ;)
It should be fixed since mono-5.16.0.235.

BTW, in case it's useful, my command for testing on Linux is:

LC_ALL=de_DE _build/ckan.exe

Thanks, that might be easier than hardcoding the CultureInfo manually and recompile. Just Windows doesn't offer an option, as always.

@HebaruSan
Copy link
Member Author

I recognize the issues I fixed ;)

Oh nice! I didn't even think to check if that was you. 😆 Congrats on getting that merged!

@DasSkelett
Copy link
Member

This is where Resources.de-DE.resx broke, it needs to be a SingleAssemblyResourceManager object or it won't check the satellite assemblies bundled in the EXE:

https://github.com/DasSkelett/CKAN/blob/ceae2c58b7f70881fc7eaa0921af0a14f871da9d/GUI/Properties/Resources.Designer.cs#L42

Can you revert all your changes to that file? You should be able to use my version without changes.

Will try, thanks 🎉.
Mind if I force-push my branch? Easiest for me is resetting the commit and unstaging the file.

@HebaruSan
Copy link
Member Author

Mind if I force-push my branch?

Please do, I won't be making changes to your branch.

@DasSkelett
Copy link
Member

Regarding the LinkArea bug (short summary for those reading this and don't want to check the linked PR: Mono doesn't (yet) take text alignment into account when rendering LinkLabels, which makes all non-upper-left aligned LinkLabels unreadable):

It should be fixed since mono-5.16.0.235.

That's wrong it seems, since I run 5.20.1.19which is from April 11th and my PR is not included. So I have no idea how the release mechanism of mono works, and when a PR is included.

Why is this bug a problem for us/me?
I changed the layout of the AboutDialogin my branch so that every (link)label spans from left to right (excluding the last row with three LinkLabels) and set

linkLabel.TextAlign = ContentAlignment.MiddleCenter

for easy centering in every language.
But with the mono bug the dialog is unreadable, most links are unclickable.
Basically, the AboutDialog is unusable like this.
If we stick to this, we must hope that the next mono release includes that fix and everyone updates to it.

A workaround/how it was before: Set the location of every link label for every language (by now only two for this form) manually so they appear centered (which is very fiddly to do!).
But at least you can read something like this.
Another idea: Calculate the string size during runtime and set the position and size accordingly. A bit dirty, but should do it.

@DasSkelett
Copy link
Member

Okay, I just found out that the icon problem is not introduced by our changes!
Other CKAN versions (i.e. current master@75deab0) have the same problems, at least on my system. It just never noticed it. But as I tested if you can remove the icon of the resource files, I obviously put attention there and spotted it.

@DasSkelett
Copy link
Member

From my side only the issue with the wrapping menu entry is left now.

grafik

@HebaruSan
Copy link
Member Author

the wrapping menu entry

This issue is unreal! There's nothing different in how we treat that object, but that effect is completely consistent and persistent no matter what changes I make.

... I think we're looking at a one-in-a-million Mono bug related to the exact pixel length of the resource string. I tried changing it to "afsdkamjsldk fdjklpa" (same number of characters with same grouping, but the letters aren't the exact same width) and it started working normally. But as long as the exact text is "Empfehlungen ansehen", it goes weird. Can you think of a slightly different translation for that string?

@DasSkelett
Copy link
Member

"Empfehlungen installierter Mods", translated to "Recommendations of installed mods" should fit well.

@HebaruSan
Copy link
Member Author

👍
Mono is so weird.

image

@politas
Copy link
Member

politas commented May 15, 2019

So we're all good to merge this, now? Are we sure those weird alignment errors don't appear with different app window sizes, or something like that?

@DasSkelett
Copy link
Member

DasSkelett commented May 15, 2019

So we're all good to merge this, now?

@HebaruSan needs to rebase once more for #2762.
@HebaruSan, if you want to include the German translation for the new string yourself, I suggest "Durchsuche GameData nach DLCs und manuell installierten Modulen...".

Are we sure those weird alignment errors don't appear with different app window sizes, or something like that?

I don't remember if I tried resizing the window, but I doubt that would affect the issue. Resizing the dropdown menu at least didn't fix it.
I can try it out tomorrow.

So after the rebase you have a Go from my side 👍.

@HebaruSan
Copy link
Member Author

HebaruSan commented May 16, 2019

Rebased!

I suggest "Durchsuche GameData nach DLCs und manuell installierten Modulen...".

I decided the English string sounded better without GameData. Does "Durchsuche nach DLCs und manuell installierten Modulen..." make sense?

@DasSkelett
Copy link
Member

"Suche nach DLCs und manuell installierten Modulen..." (without the "Durch") would be a bit more natural.

@HebaruSan
Copy link
Member Author

@politas, yes, this is ready now. I haven't merged it because I wasn't sure whether to count it as "reviewed" given that both @DasSkelett and I have made commits to the branch.

@DasSkelett
Copy link
Member

I don't remember if I tried resizing the window, but I doubt that would affect the issue. Resizing the dropdown menu at least didn't fix it.
I can try it out tomorrow.

No, resizing the window does indeed nothing. Would have been weird, because I'm sure @HebaruSan doesn't use the exact same window size, and it happened on his system too.

@Olympic1
Copy link
Member

Is it possible to add the translation files into a separate folder like en-US and de-DE, because every translation wil add 18 new files. This way it would be easier to find a specific language file and will prevent the GUI folder to get cluttered with .resx files.

@HebaruSan
Copy link
Member Author

Good idea. Rebased to get the NuGet updates and moved those to a Localization/de-DE folder. I couldn't figure out how to make it work with Resources.de-DE.resx, but those are already in the Properties folder so that shouldn't be as much of a problem.

@Olympic1
Copy link
Member

For me this is good to merge. If nobody finds any alignment errors anymore for the German translation, this can be merged.

@HebaruSan HebaruSan merged commit 26328c8 into KSP-CKAN:master Jun 10, 2019
HebaruSan added a commit that referenced this pull request Jun 10, 2019
@HebaruSan HebaruSan deleted the feature/i18n branch June 10, 2019 20:06
@DasSkelett DasSkelett added the i18n Issues regarding the internationalization of CKAN and PRs that need translating label Jan 18, 2021
@HebaruSan HebaruSan mentioned this pull request Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Issues affecting the build system Enhancement New features or functionality GUI Issues affecting the interactive GUI i18n Issues regarding the internationalization of CKAN and PRs that need translating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants